-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: Add factory for AppInstallPage #12581
Feature: Add factory for AppInstallPage #12581
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @AdalbertoMoz, thanks so much for taking on this work! 🙌
I gave the code a look and ran inv new-db,
and can confirm that everything is working perfectly.
I just have 3 small pieces of feedback that I think will wrap up this factory nicely.
If you need any help or have questions, feel free to reach out. I'd be happy to work through it together 😄
network-api/networkapi/wagtailpages/factory/youtube_regrets_page.py
Outdated
Show resolved
Hide resolved
network-api/networkapi/wagtailpages/factory/youtube_regrets_page.py
Outdated
Show resolved
Hide resolved
network-api/networkapi/wagtailpages/factory/youtube_regrets_page.py
Outdated
Show resolved
Hide resolved
Hey @danielfmiranda, thank you so much for your detailed PR review! Your feedback is always incredibly valuable. I've implemented your suggested changes in this branch and it's now ready for another review. 🚀 Note: I just changed the PR description to include the new Review App link where you can find the latest version of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @AdalbertoMoz 🙌! I just tested out the latest changes and can confirm everything is working great. LGTM! 🚀
Description
The code below introduces a Factory for the
AppInstallPage
, serving as a replacement for theYoutubeRegretsReporterExtensionPage
, which was removed in a previous pull request -> #12573The following fields were populated with fake data:
For further details, please refer to the code.
Test
This the link that you can visit to review the AppInstallPage that was created with the Factory
Link to sample test page: https://foundation-s-feature-12-cislwu.herokuapp.com/en/youtube/regretsreporter/
Related PRs/issues: #12218
Checklist
Tests
Changes in Models:
Documentation:
Merge Method
💡❗Remember to use squash merge when merging non-feature branches into
main
┆Issue is synchronized with this Jira Story